Thanks @flynd. We will need a way to test the configuration options in CI.
Adding a new example based on the monobuild example seems to be the easiest path.

Thanks @flynd. We will need a way to test the configuration options in CI. Adding a new example based on the monobuild example seems to be the easiest path.

I started looking at this, but the standard examples create a signature and then verifies it. When building without signature creation the example code needs to do something else so I'm not sure what would be appropriate here.

Thanks @flynd. We will need a way to test the configuration options in CI. Adding a new example based on the monobuild example seems to be the easiest path.

I started looking at this, but the standard examples create a signature and then verifies it. When building without signature creation the example code needs to do something else so I'm not sure what would be appropriate here.

We already have https://github.com/pq-code-package/mldsa-native/blob/main/examples/basic/expected_signatures.h. This could be extended also with keys, then you could implement keygen, sign, and verify separately.
Probably we do not want to touch existing examples as it would make them hard to understand, but for a new example, this should be fine.
I can take a look at some point, but this month is rather busy.

@mkannwischer : I've added an example that is pretty close to the actual configuration I'm using (except I omitted MLD_CONFIG_REDUCE_RAM in the example).

@mkannwischer: It's been a month. Is there any chance that you might have time to look at this now? Or is there something more that I should try to add to make it easier/better?

@mkannwischer: It's been a month. Is there any chance that you might have time to look at this now? Or is there something more that I should try to add to make it easier/better?

Apologies for the long delay! We've been busy with getting everything ready for our RWC talk.
I'll take a look this weekend.

I checked the updates you made and found a condition that had been lost, causing mld_rej_uniform_eta_table to be included even if key generation is disabled.
I pushed an update to restore it, and also to add a missing condition for the mld_polyz_unpack_XX_indices tables.

@mkannwischer : I have rebased the stack and cleaned up the commits by incorporating your formatting changes into my commits. The only difference from the previous stack is that I've re-added #if conditions around some tables that were lost in the formatting fix.

@mkannwischer : I have rebased the stack and cleaned up the commits by incorporating your formatting changes into my commits. The only difference from the previous stack is that I've re-added #if conditions around some tables that were lost in the formatting fix.

Thanks for your help @flynd - I ran out of time when I looked at this last time.
There were still some more lint errors (format had to be applied to some files, and `autogen had to be adapted to generate the correct guards) that I have fixed by now.

This PR adds quite a bit of additional pre-processor conditionals that make maintanance harder, but at the moment I don't see a better way to achieve the same, and I quite like the feature to disable APIs.
I support this change, but hope that we can simplify things in follow-up PRs. For example, #765 would help a bit already, I believe. What do you think about that one @flynd?

What is left is to run full CI (which only works for PRs from local branches) - I have created #1000 for that . We'll also need approval by @hanno-becker.

@flynd thanks for your many contributions so far! Would you like me to add you as a contributor to mldsa-native/mlkem-native? Then you could create local branches and we save this extra step above. That would also make merging easier as we do not need to involve both @hanno-becker and me for simple changes.

Closing this in favour of #1000 to avoid confusion.

This PR adds quite a bit of additional pre-processor conditionals that make maintanance harder, but at the moment I don't see a better way to achieve the same, and I quite like the feature to disable APIs. I support this change, but hope that we can simplify things in follow-up PRs. For example, #765 would help a bit already, I believe. What do you think about that one @flynd?

I agree that splitting sign.c into separate files is probably a good idea, but as you've already pointed out there is a lot of code outside sign.c that is also specific to only some of the main operations so there would still remain a lot of ifdefs.

What is left is to run full CI (which only works for PRs from local branches) - I have created #1000 for that . We'll also need approval by @hanno-becker.

I was wondering why you were always duplicating my PRs. I have not worked much with github so thank you for explaining the reason behind this.

@flynd thanks for your many contributions so far! Would you like me to add you as a contributor to mldsa-native/mlkem-native? Then you could create local branches and we save this extra step above. That would also make merging easier as we do not need to involve both @hanno-becker and me for simple changes.

I don't have anything else large after this one, but it isn't unlikely that I'll find something more minor that I want to contribute so @mkannwischer please add me so it can be done without the extra steps.

What I have locally right now is declaring all the tables as static to hide the linker symbols from other parts of the build. I'm not sure if it is okay to use MLD_CONFIG_INTERNAL_API_QUALIFIER for things that aren't functions so I don't know if I should try to push that.
The other thing I have i an ugly hack in rej_uniform_asm.S to not use stack memory that I definitely shouldn't share. =)
However I do see some cases of multiple "ldr qXX" that could be shortened by using ldp or ld1 so I will probably do that.

This PR adds quite a bit of additional pre-processor conditionals that make maintanance harder, but at the moment I don't see a better way to achieve the same, and I quite like the feature to disable APIs. I support this change, but hope that we can simplify things in follow-up PRs. For example, #765 would help a bit already, I believe. What do you think about that one @flynd?

I agree that splitting sign.c into separate files is probably a good idea, but as you've already pointed out there is a lot of code outside sign.c that is also specific to only some of the main operations so there would still remain a lot of ifdefs.

Maybe further splitting is called for later, but starting with sign.c sounds like a good idea.

What is left is to run full CI (which only works for PRs from local branches) - I have created #1000 for that . We'll also need approval by @hanno-becker.

I was wondering why you were always duplicating my PRs. I have not worked much with github so thank you for explaining the reason behind this.

It's actually the AWS EC2 auth that doesn't work from forks as the auth tokens are not available. This may be a configuration issue (only the Linux Foundation staff can change the org configuration), but we have not yet found the time to look into it. It feels like we will soon have to do it because this approach clearly doesn't scale.

@flynd thanks for your many contributions so far! Would you like me to add you as a contributor to mldsa-native/mlkem-native? Then you could create local branches and we save this extra step above. That would also make merging easier as we do not need to involve both @hanno-becker and me for simple changes.

I don't have anything else large after this one, but it isn't unlikely that I'll find something more minor that I want to contribute so @mkannwischer please add me so it can be done without the extra steps.

What I have locally right now is declaring all the tables as static to hide the linker symbols from other parts of the build. I'm not sure if it is okay to use MLD_CONFIG_INTERNAL_API_QUALIFIER for things that aren't functions so I don't know if I should try to push that. The other thing I have i an ugly hack in rej_uniform_asm.S to not use stack memory that I definitely shouldn't share. =) However I do see some cases of multiple "ldr qXX" that could be shortened by using ldp or ld1 so I will probably do that.

Nice, I have added you in pq-code-package/tsc#207 and you should have gotten an e-mail about it. This should also allow you to make changes to #1000. Feel free to do that.

Nice, I have added you in pq-code-package/tsc#207 and you should have gotten an e-mail about it. This should also allow you to make changes to #1000. Feel free to do that.

Thank you @mkannwischer. I was able to push an update for that PR, but the CI gets the same "nix: command not found" as when I pushed via my forked repo so something more seems to be needed.

Nice, I have added you in pq-code-package/tsc#207 and you should have gotten an e-mail about it. This should also allow you to make changes to #1000. Feel free to do that.

Thank you @mkannwischer. I was able to push an update for that PR, but the CI gets the same "nix: command not found" as when I pushed via my forked repo so something more seems to be needed.

The nix: command not found warning is actually expected here - that happens when it checks if nix is installed and installs it when it isn't. Can be improved to not result in warning.

The actual error is:

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:36  Done ✓
  error mldsa/src/fips202/native/aarch64/auto.h
  Autogenerated file mldsa/src/fips202/native/aarch64/auto.h needs updating. Have 
  you called scripts/autogen? Wrote new version to 
  mldsa/src/fips202/native/aarch64/auto.h.new.
  40,41c40,42
  < #if (!defined(MLD_CONFIG_NO_KEYPAIR_API) || !defined(MLD_CONFIG_NO_SIGN_API) || \
  <      !defined(MLD_CONFIG_REDUCE_RAM)) && !defined(MLD_CONFIG_SERIAL_FIPS202_ONLY)
  ---
  > #if (!defined(MLD_CONFIG_NO_KEYPAIR_API) ||                                  \
  >      !defined(MLD_CONFIG_NO_SIGN_API) || !defined(MLD_CONFIG_REDUCE_RAM)) && \
  >     !defined(MLD_CONFIG_SERIAL_FIPS202_ONLY)
  error dev/fips202/aarch64/auto.h
  Autogenerated file dev/fips202/aarch64/auto.h needs updating. Have you called 
  scripts/autogen? Wrote new version to dev/fips202/aarch64/auto.h.new.
  40,41c40,42
  < #if (!defined(MLD_CONFIG_NO_KEYPAIR_API) || !defined(MLD_CONFIG_NO_SIGN_API) || \
  <      !defined(MLD_CONFIG_REDUCE_RAM)) && !defined(MLD_CONFIG_SERIAL_FIPS202_ONLY)
  ---
  > #if (!defined(MLD_CONFIG_NO_KEYPAIR_API) ||                                  \
  >      !defined(MLD_CONFIG_NO_SIGN_API) || !defined(MLD_CONFIG_REDUCE_RAM)) && \
  >     !defined(MLD_CONFIG_SERIAL_FIPS202_ONLY)
  ✗ Check native auto-generated files

Which suggestts you have not run autogen after you made your changes.
You should be able to run lint locally to reproduce the same error.

Nice, I have added you in pq-code-package/tsc#207 and you should have gotten an e-mail about it. This should also allow you to make changes to #1000. Feel free to do that.

Thank you @mkannwischer. I was able to push an update for that PR, but the CI gets the same "nix: command not found" as when I pushed via my forked repo so something more seems to be needed.

The nix: command not found warning is actually expected here - that happens when it checks if nix is installed and installs it when it isn't. Can be improved to not result in warning.

Which suggestts you have not run autogen after you made your changes. You should be able to run lint locally to reproduce the same error.

Ah, I see. I opened the job page and the nix error was the only thing that stood out as an error there so I assumed that was what the issue that was causing it to fail.

I have managed to run the autogen script locally now. I got some weirdness in unrelated files that I will assume is due to my slightly outdated Debian system, but I at least got the formatting changes for the ifdefs so I could fix those and the lint test has passed this time.

Thank you again for explaining.

Ah, I see. I opened the job page and the nix error was the only thing that stood out as an error there so I assumed that was what the issue that was causing it to fail.

Yes, the output of the lint script is really not great in CI :( Sorry. I have opened pq-code-package/mlkem-native#1638 to improve this.

I have managed to run the autogen script locally now. I got some weirdness in unrelated files that I will assume is due to my slightly outdated Debian system, but I at least got the formatting changes for the ifdefs so I could fix those and the lint test has passed this time.

Nice!
Is that clang-format messing with C files? You would have to install the same clang-format version. We all use the nix flake which ensures we all have the same versions of everything (that's particularly important for the CBMC and HOL-Light proofs).
If it's not clang-format, I'd be interested to hear what weirdness you have experienced.

I have managed to run the autogen script locally now. I got some weirdness in unrelated files that I will assume is due to my slightly outdated Debian system, but I at least got the formatting changes for the ifdefs so I could fix those and the lint test has passed this time.

Nice! Is that clang-format messing with C files? You would have to install the same clang-format version. We all use the nix flake which ensures we all have the same versions of everything (that's particularly important for the CBMC and HOL-Light proofs). If it's not clang-format, I'd be interested to hear what weirdness you have experienced.

The large diff I got was a bunch of assembly files that changed hex constants to decimal values. I'm not sure if that is clang-format or something else causing it.
I only used the Debian package manager to bring in the commands that autogen complained about until it stopped failing, so clang-format is 19.1.7. I haven't looked yet at what nix is or what it does.

I have managed to run the autogen script locally now. I got some weirdness in unrelated files that I will assume is due to my slightly outdated Debian system, but I at least got the formatting changes for the ifdefs so I could fix those and the lint test has passed this time.

Nice! Is that clang-format messing with C files? You would have to install the same clang-format version. We all use the nix flake which ensures we all have the same versions of everything (that's particularly important for the CBMC and HOL-Light proofs). If it's not clang-format, I'd be interested to hear what weirdness you have experienced.

The large diff I got was a bunch of assembly files that changed hex constants to decimal values. I'm not sure if that is clang-format or something else causing it. I only used the Debian package manager to bring in the commands that autogen complained about until it stopped failing, so clang-format is 19.1.7. I haven't looked yet at what nix is or what it does.

Ahh, no then it's not clang-format. It's our simpasm script that assembles the asm in dev/ and objdumps it again into mldsa/ (this is needed because some of our consumers can't handle assembly macros and other GAS features).
So this is most likely due to a different version of your assembler. If it's just hex constants, I wouldn't worry about it. If there is more serious changes, then it's worth to investigate.

This is also related to our reassembly soundness risk that the object code that is verified in HOL-Light may be different from the object code your assembler actually produces (see https://github.com/pq-code-package/mlkem-native/blob/ddbc21d20d08467541317e3c501693fb0d9c309e/SOUNDNESS.md?plain=1#L389 and also the corresponding issue pq-code-package/mlkem-native#1602).

As for nix: It's a package manager that allows us to describe all dev dependencies in a reproducible way. You just type nix develop and you have the same setup as we do. We make extensive use of this in the CI. We also have a bunch of other environment setups, e.g., nix develop .#cross-riscv64 gives you a RV64 cross compiler.

Installation is straightforward: https://nixos.org/download/
On Linux you just run:

sh <(curl --proto '=https' --tlsv1.2 -L https://nixos.org/nix/install) --daemon